Catch and guard against Integration errors #66
Conversation
|
Rough plan here is:
|
f2prateek
requested review from
dominicbarnes,
tamarrow-zz,
CarlosMecha and
Peripheral1994
|
How come we're moving the catching of integration errors into analytics.js-core? |
|
Mentioned this in the SDD (https://paper.dropbox.com/doc/Analytics.js-Metrics-SDD-1hAD90lqGS4aZxHAHYPu7?t=966581820045516849090263#:t=966581820045516849090263), but the TLDR is that current analytics.js-integration swallows errors, so to record metrics for those we would need to either: I went with b) as it limits the metrics instrumentation to one repo (a.js-core) which should be easier for both the initial implementation and long term maintenance. |
|
Cool, makes sense! Thanks for clarifying. So this PR is so that we can next remove a corresponding |
|
I would approve this PR, but I don't think I know enough about these critical pieces to do so. So I'll defer to others who know more about A.js internals. |
Yep! |
As part of adding client side metrics, we want to propagate errors to analytics.js-core so that it can capture them. analytics.js-core has already been updated to capture errors when invoking integrations (segmentio/analytics.js-core#66).
To be able to record client side metrics, we need to modify analytics.js-integration to start propagate errors to analytics.js-core (https://paper.dropbox.com/doc/Analytics.js-Metrics-SDD-1hAD90lqGS4aZxHAHYPu7#:h2=analytics.js-integration) so that analytics.js-core can record metrics. Currently analytics.js-integration is responsible for guarding when an integration method throws an error. Before changing the behaviour in analytics.js-integration, we need to make sure analytics.js-core guards against errors in integrations.
f2prateek
force-pushed
the
catch-integration-errors
branch
from
3ab777c to
1109064
Compare
As part of adding client side metrics, we want to propagate errors to analytics.js-core so that it can capture them. https://paper.dropbox.com/doc/Analytics.js-Metrics-SDD-1hAD90lqGS4aZxHAHYPu7#:h2=analytics.js-integration analytics.js-core has already been updated to capture errors when invoking integrations (segmentio/analytics.js-core#66).
As part of adding client side metrics, we want to propagate errors to analytics.js-core so that it can capture them. https://paper.dropbox.com/doc/Analytics.js-Metrics-SDD-1hAD90lqGS4aZxHAHYPu7#:h2=analytics.js-integration analytics.js-core has already been updated to capture errors when invoking integrations (segmentio/analytics.js-core#66).
As part of adding client side metrics, we want to propagate errors to analytics.js-core so that it can capture them. https://paper.dropbox.com/doc/Analytics.js-Metrics-SDD-1hAD90lqGS4aZxHAHYPu7#:h2=analytics.js-integration analytics.js-core has already been updated to capture errors when invoking integrations (segmentio/analytics.js-core#66).
|
FYI build failures are the same ones that have been failing even on master (e.g. https://circleci.com/gh/segmentio/analytics.js-core/329). I'll look into fixing that seperately, but keep that in mind! |
Same as #60 but for 2.x. As part of adding client side metrics, we want to propagate errors to analytics.js-core so that it can capture them. https://paper.dropbox.com/doc/Analytics.js-Metrics-SDD-1hAD90lqGS4aZxHAHYPu7#:h2=analytics.js-integration analytics.js-core has already been updated to capture errors when invoking integrations (segmentio/analytics.js-core#66).
To be able to record client side metrics, we need to modify analytics.js-integration to start propagate errors to analytics.js-core (https://paper.dropbox.com/doc/Analytics.js-Metrics-SDD-1hAD90lqGS4aZxHAHYPu7#:h2=analytics.js-integration) so that analytics.js-core can record metrics.
Currently analytics.js-integration is responsible for guarding when an integration method throws an error. Before changing the behaviour in analytics.js-integration, we need to make sure analytics.js-core guards against errors in integrations.